Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JS] Remove role='menubar' from ActionCollection #6763

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

paulcam206
Copy link
Member

@paulcam206 paulcam206 commented Nov 23, 2021

Related Issue

Fixes #6425
Fixes #6019

Description

Remove the menubar role from ActionCollection (as well as aria-posinset, aria-setsize, and role='menuitem' from children). This was originally part of a fix to expose actions in a set to address a different accessibility concern (see #4184) -- namely that actions in an ActionSet should be presented in such a way that AT can tell the user how many actions there are. The way you do that is via aria-posinset and aria-setsize, which are only applicable to a limited number of roles (button and link not being among them). There can, of course, only be one role applied to an element, so we must unfortunately choose to stick with button or link and remove the other attributes.

How Verified

  • local build, devtools, narrator
Microsoft Reviewers: Open in CodeFlow

@ghost
Copy link

ghost commented Nov 23, 2021

Hi @paulcam206. Thanks for helping make the AdaptiveCards JS renderer + tooling better. As additional verification, once the JS build succeeds, please go to the test site to test out your website/designer changes.

Copy link
Member

@jwoo-msft jwoo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should we reopen #4184 since this change undoes fix for #4184?

@paulcam206
Copy link
Member Author

nit: should we reopen #4184 since this change undoes fix for #4184?

nah -- there's really no way to achieve what they wanted with what we have available to us

@compulim
Copy link
Contributor

compulim commented Jul 1, 2022

(I know not many people will read comments in merged/closed PRs. But I will just keep it here.)

IMO, role="toolbar" is a better role for grouping buttons. Some screen readers read "1 of 6" even without aria-posinset for things inside role="toolbar".

Turning a button into menu item hurts accessibility as shortcut keys that normally applies to buttons (and form fields) will no longer apply to menu item. Say, pressing B or F in Narrator will jump to the next closest button. However, it won't jump to button[role="menuitem"].

Buttons in toolbar are still buttons. Buttons in menu bar are no longer buttons. Better to stay as close to its original form as possible.

licanhua added a commit that referenced this pull request Sep 14, 2022
* Updating Bot Framework links (#6686)

Co-authored-by: RahulAmlekar <[email protected]>

* Scenario cards + Website indentation fix (#6733)

* Changed teams target version to 1.4

* added RTL scenario card

* Added scenario card for tooltip and isEnabled

* Added order confirmation scenario

* Added flight update table scenario

* Added restaurant order scenario card

* Added app login scenario

* Fixed indentation in schema explorer

* Testing application login on the samples page

* Added extended 1.5 scenario cards

* Fixed boolean and duplicate id on cards

* added flightupdatetable and restaurant order to ignored cards for .NET tests

Co-authored-by: Canhua Li <[email protected]>

* Removing "Preview" from Viva Connections (#6791)

Co-authored-by: Paul Campbell <[email protected]>

* Added blog posts for oct and nov (#6873)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

Co-authored-by: Canhua Li <[email protected]>
Co-authored-by: almedina-ms <[email protected]>

* make columnsets clickable (#7042)

Co-authored-by: Canhua Li <[email protected]>

* fix merge conflicts with berlin containers in adaptivecards-designer.ts

* Convey Ctrl+M shortcut key to move out of code editor (#7072)

* provide hint about ctrl+m toggling tab behavior

* rephrase

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Canhua Li <[email protected]>

* fixed same merge conflict in another commit

* color fix (#7081)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update headers (#7082)

* Add title to iframe component in December 2019 blog post (#7083)

* add title

* spacing

Co-authored-by: Pankaj Bhojwani <[email protected]>

* change text color (#7091)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add aria label (#7090)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update tab indices accordingly (#7119)

Co-authored-by: Vsevolod <[email protected]>

* Adjust contrast on designer peer buttons (#7110)

* Update role of 'Select Host App' combobox (#7118)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Delete old samples that don't have latest features (causing accessibility issues) (#7106)

* updates to samples

* phone number regex

* delete files instead

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* removed carousel samples

* fixed merge conflict line 8 of package.json

* fix titles (#7130)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* fixed merge conflict in ADCIOSVisualizerUITests

* add tooltip on initialization (#7126)

* more titles to iframes (#7135)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add alt text (#7131)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* use active border instead of white (#7122)

* add landmark (#7145)

* Use HTML lists for hyperlinks (#7127)

* Use HTML lists for hyperlinks

* consistent spacing in css file

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>

* Add a caption to the table present in the 'help' dialog (#7149)

* move keyboard shortcut text to caption

* use innerText instead

* don't let focus fall on card designer surface (#7153)

* add 1 px left margin (#7142)

* Add underline styling to hyperlinks (#7157)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Allow footer hyperlinks to wrap (#7147)

Co-authored-by: PankajBhojwani <[email protected]>

* Website updates (#7163)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

* added blogs posts

* fixed video titles

* updated roadmap links

* removed repeated sentence

Co-authored-by: Canhua Li <[email protected]>

* [JS] Remove `role='menubar'` and `role='menuitem'` from `ActionCollection` (#6763)

Fixes #6425
Fixes #6019
Related #4859

* sync loc file

* more package-locks

* cherry pick 646503b

* return focus after closing (#7155)

* final cherry picks for 5e0f39f and 67187d0

* update designer

* revert package-lock.json changes

Co-authored-by: Rahul Amlekar <[email protected]>
Co-authored-by: RahulAmlekar <[email protected]>
Co-authored-by: J.P. Roca <[email protected]>
Co-authored-by: Siddharth Gulati <[email protected]>
Co-authored-by: Paul Campbell <[email protected]>
Co-authored-by: almedina-ms <[email protected]>
Co-authored-by: Roy Nehoran <[email protected]>
Co-authored-by: naramaka <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>
Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Will Shown <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: Carlos Zamora <[email protected]>
Co-authored-by: root <[email protected]>
licanhua added a commit that referenced this pull request Sep 15, 2022
* Updating Bot Framework links (#6686)

Co-authored-by: RahulAmlekar <[email protected]>

* Scenario cards + Website indentation fix (#6733)

* Changed teams target version to 1.4

* added RTL scenario card

* Added scenario card for tooltip and isEnabled

* Added order confirmation scenario

* Added flight update table scenario

* Added restaurant order scenario card

* Added app login scenario

* Fixed indentation in schema explorer

* Testing application login on the samples page

* Added extended 1.5 scenario cards

* Fixed boolean and duplicate id on cards

* added flightupdatetable and restaurant order to ignored cards for .NET tests

Co-authored-by: Canhua Li <[email protected]>

* Removing "Preview" from Viva Connections (#6791)

Co-authored-by: Paul Campbell <[email protected]>

* Added blog posts for oct and nov (#6873)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

Co-authored-by: Canhua Li <[email protected]>
Co-authored-by: almedina-ms <[email protected]>

* make columnsets clickable (#7042)

Co-authored-by: Canhua Li <[email protected]>

* fix merge conflicts with berlin containers in adaptivecards-designer.ts

* Convey Ctrl+M shortcut key to move out of code editor (#7072)

* provide hint about ctrl+m toggling tab behavior

* rephrase

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Canhua Li <[email protected]>

* fixed same merge conflict in another commit

* color fix (#7081)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update headers (#7082)

* Add title to iframe component in December 2019 blog post (#7083)

* add title

* spacing

Co-authored-by: Pankaj Bhojwani <[email protected]>

* change text color (#7091)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add aria label (#7090)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update tab indices accordingly (#7119)

Co-authored-by: Vsevolod <[email protected]>

* Adjust contrast on designer peer buttons (#7110)

* Update role of 'Select Host App' combobox (#7118)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Delete old samples that don't have latest features (causing accessibility issues) (#7106)

* updates to samples

* phone number regex

* delete files instead

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* removed carousel samples

* fixed merge conflict line 8 of package.json

* fix titles (#7130)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* fixed merge conflict in ADCIOSVisualizerUITests

* add tooltip on initialization (#7126)

* more titles to iframes (#7135)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add alt text (#7131)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* use active border instead of white (#7122)

* add landmark (#7145)

* Use HTML lists for hyperlinks (#7127)

* Use HTML lists for hyperlinks

* consistent spacing in css file

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>

* Add a caption to the table present in the 'help' dialog (#7149)

* move keyboard shortcut text to caption

* use innerText instead

* don't let focus fall on card designer surface (#7153)

* add 1 px left margin (#7142)

* Add underline styling to hyperlinks (#7157)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Allow footer hyperlinks to wrap (#7147)

Co-authored-by: PankajBhojwani <[email protected]>

* Website updates (#7163)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

* added blogs posts

* fixed video titles

* updated roadmap links

* removed repeated sentence

Co-authored-by: Canhua Li <[email protected]>

* [JS] Remove `role='menubar'` and `role='menuitem'` from `ActionCollection` (#6763)

Fixes #6425
Fixes #6019
Related #4859

* sync loc file

* more package-locks

* cherry pick 646503b

* return focus after closing (#7155)

* final cherry picks for 5e0f39f and 67187d0

* update designer

* revert package-lock.json changes

Co-authored-by: Rahul Amlekar <[email protected]>
Co-authored-by: RahulAmlekar <[email protected]>
Co-authored-by: J.P. Roca <[email protected]>
Co-authored-by: Siddharth Gulati <[email protected]>
Co-authored-by: Paul Campbell <[email protected]>
Co-authored-by: almedina-ms <[email protected]>
Co-authored-by: Roy Nehoran <[email protected]>
Co-authored-by: naramaka <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>
Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Will Shown <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: Carlos Zamora <[email protected]>
Co-authored-by: root <[email protected]>
licanhua added a commit that referenced this pull request Sep 15, 2022
#7873)

* Merge website march22 to main branch (#7857)

* Updating Bot Framework links (#6686)

Co-authored-by: RahulAmlekar <[email protected]>

* Scenario cards + Website indentation fix (#6733)

* Changed teams target version to 1.4

* added RTL scenario card

* Added scenario card for tooltip and isEnabled

* Added order confirmation scenario

* Added flight update table scenario

* Added restaurant order scenario card

* Added app login scenario

* Fixed indentation in schema explorer

* Testing application login on the samples page

* Added extended 1.5 scenario cards

* Fixed boolean and duplicate id on cards

* added flightupdatetable and restaurant order to ignored cards for .NET tests

Co-authored-by: Canhua Li <[email protected]>

* Removing "Preview" from Viva Connections (#6791)

Co-authored-by: Paul Campbell <[email protected]>

* Added blog posts for oct and nov (#6873)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

Co-authored-by: Canhua Li <[email protected]>
Co-authored-by: almedina-ms <[email protected]>

* make columnsets clickable (#7042)

Co-authored-by: Canhua Li <[email protected]>

* fix merge conflicts with berlin containers in adaptivecards-designer.ts

* Convey Ctrl+M shortcut key to move out of code editor (#7072)

* provide hint about ctrl+m toggling tab behavior

* rephrase

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Canhua Li <[email protected]>

* fixed same merge conflict in another commit

* color fix (#7081)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update headers (#7082)

* Add title to iframe component in December 2019 blog post (#7083)

* add title

* spacing

Co-authored-by: Pankaj Bhojwani <[email protected]>

* change text color (#7091)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add aria label (#7090)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update tab indices accordingly (#7119)

Co-authored-by: Vsevolod <[email protected]>

* Adjust contrast on designer peer buttons (#7110)

* Update role of 'Select Host App' combobox (#7118)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Delete old samples that don't have latest features (causing accessibility issues) (#7106)

* updates to samples

* phone number regex

* delete files instead

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* removed carousel samples

* fixed merge conflict line 8 of package.json

* fix titles (#7130)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* fixed merge conflict in ADCIOSVisualizerUITests

* add tooltip on initialization (#7126)

* more titles to iframes (#7135)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add alt text (#7131)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* use active border instead of white (#7122)

* add landmark (#7145)

* Use HTML lists for hyperlinks (#7127)

* Use HTML lists for hyperlinks

* consistent spacing in css file

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>

* Add a caption to the table present in the 'help' dialog (#7149)

* move keyboard shortcut text to caption

* use innerText instead

* don't let focus fall on card designer surface (#7153)

* add 1 px left margin (#7142)

* Add underline styling to hyperlinks (#7157)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Allow footer hyperlinks to wrap (#7147)

Co-authored-by: PankajBhojwani <[email protected]>

* Website updates (#7163)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

* added blogs posts

* fixed video titles

* updated roadmap links

* removed repeated sentence

Co-authored-by: Canhua Li <[email protected]>

* [JS] Remove `role='menubar'` and `role='menuitem'` from `ActionCollection` (#6763)

Fixes #6425
Fixes #6019
Related #4859

* sync loc file

* more package-locks

* cherry pick 646503b

* return focus after closing (#7155)

* final cherry picks for 5e0f39f and 67187d0

* update designer

* revert package-lock.json changes

Co-authored-by: Rahul Amlekar <[email protected]>
Co-authored-by: RahulAmlekar <[email protected]>
Co-authored-by: J.P. Roca <[email protected]>
Co-authored-by: Siddharth Gulati <[email protected]>
Co-authored-by: Paul Campbell <[email protected]>
Co-authored-by: almedina-ms <[email protected]>
Co-authored-by: Roy Nehoran <[email protected]>
Co-authored-by: naramaka <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>
Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Will Shown <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: Carlos Zamora <[email protected]>
Co-authored-by: root <[email protected]>

* prefix adaptivecards#  for ttpolicy (#7860)

* remove dead code searchAndRemoveForbiddenElements

* Revert "use active border instead of white (#7122)" (#7874)

This reverts commit 68d88fe.

Co-authored-by: Rahul Amlekar <[email protected]>
Co-authored-by: RahulAmlekar <[email protected]>
Co-authored-by: J.P. Roca <[email protected]>
Co-authored-by: Siddharth Gulati <[email protected]>
Co-authored-by: Paul Campbell <[email protected]>
Co-authored-by: almedina-ms <[email protected]>
Co-authored-by: Roy Nehoran <[email protected]>
Co-authored-by: naramaka <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>
Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Will Shown <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: Carlos Zamora <[email protected]>
Co-authored-by: root <[email protected]>
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
* Updating Bot Framework links (microsoft#6686)

Co-authored-by: RahulAmlekar <[email protected]>

* Scenario cards + Website indentation fix (microsoft#6733)

* Changed teams target version to 1.4

* added RTL scenario card

* Added scenario card for tooltip and isEnabled

* Added order confirmation scenario

* Added flight update table scenario

* Added restaurant order scenario card

* Added app login scenario

* Fixed indentation in schema explorer

* Testing application login on the samples page

* Added extended 1.5 scenario cards

* Fixed boolean and duplicate id on cards

* added flightupdatetable and restaurant order to ignored cards for .NET tests

Co-authored-by: Canhua Li <[email protected]>

* Removing "Preview" from Viva Connections (microsoft#6791)

Co-authored-by: Paul Campbell <[email protected]>

* Added blog posts for oct and nov (microsoft#6873)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

Co-authored-by: Canhua Li <[email protected]>
Co-authored-by: almedina-ms <[email protected]>

* make columnsets clickable (microsoft#7042)

Co-authored-by: Canhua Li <[email protected]>

* fix merge conflicts with berlin containers in adaptivecards-designer.ts

* Convey Ctrl+M shortcut key to move out of code editor (microsoft#7072)

* provide hint about ctrl+m toggling tab behavior

* rephrase

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Canhua Li <[email protected]>

* fixed same merge conflict in another commit

* color fix (microsoft#7081)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update headers (microsoft#7082)

* Add title to iframe component in December 2019 blog post (microsoft#7083)

* add title

* spacing

Co-authored-by: Pankaj Bhojwani <[email protected]>

* change text color (microsoft#7091)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add aria label (microsoft#7090)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* update tab indices accordingly (microsoft#7119)

Co-authored-by: Vsevolod <[email protected]>

* Adjust contrast on designer peer buttons (microsoft#7110)

* Update role of 'Select Host App' combobox (microsoft#7118)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Delete old samples that don't have latest features (causing accessibility issues) (microsoft#7106)

* updates to samples

* phone number regex

* delete files instead

Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* removed carousel samples

* fixed merge conflict line 8 of package.json

* fix titles (microsoft#7130)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* fixed merge conflict in ADCIOSVisualizerUITests

* add tooltip on initialization (microsoft#7126)

* more titles to iframes (microsoft#7135)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* add alt text (microsoft#7131)

Co-authored-by: Pankaj Bhojwani <[email protected]>

* use active border instead of white (microsoft#7122)

* add landmark (microsoft#7145)

* Use HTML lists for hyperlinks (microsoft#7127)

* Use HTML lists for hyperlinks

* consistent spacing in css file

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>

* Add a caption to the table present in the 'help' dialog (microsoft#7149)

* move keyboard shortcut text to caption

* use innerText instead

* don't let focus fall on card designer surface (microsoft#7153)

* add 1 px left margin (microsoft#7142)

* Add underline styling to hyperlinks (microsoft#7157)

Co-authored-by: root <[email protected]>
Co-authored-by: Vsevolod <[email protected]>

* Allow footer hyperlinks to wrap (microsoft#7147)

Co-authored-by: PankajBhojwani <[email protected]>

* Website updates (microsoft#7163)

* Changed teams target version to 1.4

* Added october and november blog posts

* fixed emojis

* testing font weights for emoji

* removed bold from emojis as not required to render

* added blogs posts

* fixed video titles

* updated roadmap links

* removed repeated sentence

Co-authored-by: Canhua Li <[email protected]>

* [JS] Remove `role='menubar'` and `role='menuitem'` from `ActionCollection` (microsoft#6763)

Fixes microsoft#6425
Fixes microsoft#6019
Related microsoft#4859

* sync loc file

* more package-locks

* cherry pick 646503b

* return focus after closing (microsoft#7155)

* final cherry picks for 5e0f39f and 67187d0

* update designer

* revert package-lock.json changes

Co-authored-by: Rahul Amlekar <[email protected]>
Co-authored-by: RahulAmlekar <[email protected]>
Co-authored-by: J.P. Roca <[email protected]>
Co-authored-by: Siddharth Gulati <[email protected]>
Co-authored-by: Paul Campbell <[email protected]>
Co-authored-by: almedina-ms <[email protected]>
Co-authored-by: Roy Nehoran <[email protected]>
Co-authored-by: naramaka <[email protected]>
Co-authored-by: PankajBhojwani <[email protected]>
Co-authored-by: Pankaj Bhojwani <[email protected]>
Co-authored-by: Will Shown <[email protected]>
Co-authored-by: Vsevolod <[email protected]>
Co-authored-by: Carlos Zamora <[email protected]>
Co-authored-by: root <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants